Repository navigation
fix: keep colliding virtual nodes in ConsistentHash - #3611
Merged
He-Pin merged 2 commits intoOct 10, 2026
Merged
Conversation
Motivation: `ConsistentHash` stores ring positions in a `SortedMap[Int, T]`. When virtual nodes of two different nodes hash to the same position, the later entry silently overwrites the earlier one. The ring then depends on the order in which nodes were given or added, so two instances built from the same nodes can route the same key to different nodes. Removing a node also deletes any position it collides on, even if that position is owned by another node (or the removed node was never in the ring), so the remaining node loses virtual nodes. Modification: - Keep all nodes that claim a ring position: the owner in the ring, the others in a `collisions` map, which is normally empty. - On a collision the node with the lowest `toString` owns the position, independent of insertion order. - `:-` only releases the removed node's claims and promotes the next claimant if the owner is removed. - Document the collision rule in the class Scaladoc. - Add `ConsistentHashSpec` using two node names whose virtual nodes are known to collide. Result: The ring, and so `nodeFor`, is the same regardless of node order, and `ch :- node` routes like a ring built without that node. Routing only changes compared to before for keys that land on a colliding position. Tests: - `actor-tests/Test/testOnly org.apache.pekko.routing.ConsistentHashSpec` without the fix: 5 of 6 failed - `actor-tests/Test/testOnly org.apache.pekko.routing.ConsistentHashSpec org.apache.pekko.routing.ConsistentHashingRouterSpec` with the fix: 9 passed on 2.13.18 and 3.3.8 - `actor/mimaReportBinaryIssues`: passed on 2.13.18 and 3.3.8 - scalafmt run on changed files References: Fixes apache#3279
Motivation: Resolving collisions by claiming one virtual node at a time made `ConsistentHash.apply` insert into the persistent ring map point by point. Against the previous bulk `SortedMap.empty ++ pairs` build that is 2-3x the allocation and several times the time (JMH, 10 virtual nodes per node: 100 nodes 230 KB -> 576 KB, 1000 nodes 2.4 MB -> 7.0 MB). `apply` is used to rebuild the ring on membership changes by the classic consistent hashing router, `ConsistentHashingShardAllocationStrategy` and `ClusterClient`. Modification: - Encode each virtual node as `(ring position << 32 | index)` in a primitive `Long` array, sort it once, and add positions with a single claimant through a `TreeMap` builder. - Resolve only runs of equal positions (collisions) with the same owner rule as `:+`, via a shared `ownerOf` helper. Result: `apply` produces the same ring as before this commit, and allocates less than the original (pre-fix) bulk build at similar speed. JMH, 10 virtual nodes per node, original vs this commit: - 10 nodes: 24.4 KB -> 18.5 KB, 12 us -> 22 us (+-20 us, noisy) - 100 nodes: 230 KB -> 201 KB, 219 us -> 235 us - 1000 nodes: 2.39 MB -> 1.87 MB, 3378 us -> 2899 us Tests: - `actor-tests/Test/testOnly org.apache.pekko.routing.ConsistentHashSpec org.apache.pekko.routing.ConsistentHashingRouterSpec`: 10 passed on 2.13.18 and 3.3.8, including a new check that `apply` matches adding the nodes one by one with `:+` - `actor/mimaReportBinaryIssues`: passed on 2.13.18 and 3.3.8 - scalafmt run on changed files References: Refs apache#3279
Member
Author
Impact of the routing change for colliding ring positionsRouting only changes for keys that land on a ring position where virtual nodes of two different nodes collide. Everywhere else the ring is identical to How likely a collision is. Ring positions are 32-bit hashes. With P = nodes × virtual nodes positions, the expected number of colliding positions is about P²/2³³ (the default
Only keys on the arc owned by the colliding position (about 1/P of keys) can move, and only when the previous last-written-wins owner differs from the new lowest- What this means for each user, including mixed versions during a rolling upgrade:
Separately, Suggested release note: " |
He-Pin
approved these changes
Oct 10, 2026
He-Pin
left a comment
He-Pin
left a comment
Member
There was a problem hiding this comment.
LGTM. Deterministic collision ownership (lowest toString wins) is correct and well covered by the new spec: apply vs incremental equivalence, re-election on removal, and removing a node that is not in the ring. Minor non-blocking nit: the virtualNodesFactor < 1 check added in apply duplicates the one already in the class body.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
ConsistentHashstores ring positions in aSortedMap[Int, T]. When virtual nodes of two different nodes hash to the same position, the later entry silently overwrites the earlier one. As a result:Modification
Keep every node that claims a ring position: the owner stays in the ring, and the others go in a
collisionsmap, which is normally empty.On a collision, the node with the lowest
toStringowns the position, independent of insertion order. Nodes are already identified bytoString, per the class docs.:-only releases the removed node's claims, and promotes the next claimant if the owner is removed.Documented the collision rule in the class Scaladoc.
Added
ConsistentHashSpec, using the two node names from the issue whose virtual nodes are known to collide with a factor of 10.applybuilds the ring in one sorted batch: each virtual node is encoded as(ring position << 32 | index)in a primitiveLongarray and sorted once. Single-claimant positions go through aTreeMapbuilder, and only colliding runs go through the owner rule shared with:+.Performance
JMH, 10 virtual nodes per node, old = current
main, new = this PR:nodeForapplyapplyapply:+/:-nodeFor, which is on the per-message path, is unchanged.apply,:+and:-only run when the set of nodes or routees changes.Result
The ring, and so
nodeFor, is the same regardless of node order, andch :- noderoutes like a ring built without that node.Compared with the current release, routing only changes for keys that land on a colliding ring position. During a rolling upgrade, nodes on old and new versions could route those few keys differently.
Tests
actor-tests/Test/testOnly org.apache.pekko.routing.ConsistentHashSpecwithout the fix: 5 of 6 failed (the remaining one is an "empty after removing all nodes" sanity check)actor-tests/Test/testOnly org.apache.pekko.routing.ConsistentHashSpec org.apache.pekko.routing.ConsistentHashingRouterSpecwith the fix: 10 passed on 2.13.18 and 3.3.8, including a check thatapplybuilds the same ring as adding the nodes one by one with:+ConsistentHashBench(local only, not committed) comparingmainand this PR, results aboveactor/mimaReportBinaryIssues: passed on 2.13.18 and 3.3.8References
Fixes #3279